feat(CodeEditor): add insertText method #902
Open
+25
−3
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Thanks for your PR, @kimdiego2098. Someone from the team will get assigned to your PR shortly and we'll get it reviewed. |
Reviewer's GuideAdds a new text insertion API to the CodeEditor component and wires it to a corresponding JavaScript function that inserts text at the current Monaco editor selection and refocuses the editor. Sequence diagram for CodeEditor.InsertText interactionsequenceDiagram
actor User
participant BlazorCodeEditor as CodeEditorComponent
participant JSRuntime as BlazorJSInterop
participant CodeEditorJs as CodeEditor_razor_js
participant Monaco as MonacoEditor
User->>BlazorCodeEditor: Invoke InsertText(data)
BlazorCodeEditor->>JSRuntime: InvokeVoidAsync insertText Id, data
JSRuntime->>CodeEditorJs: insertText id, insertData
CodeEditorJs->>CodeEditorJs: wrapper = Data.get id
CodeEditorJs->>Monaco: editor.getSelection()
CodeEditorJs->>Monaco: executeEdits insert-custom-text, range, text
CodeEditorJs->>Monaco: focus()
Class diagram for updated CodeEditor text insertion APIclassDiagram
class CodeEditorComponent {
+string Id
+Task Resize()
+Task InsertText(string data)
}
class CodeEditor_razor_js {
+init(id, interop, options)
+insertText(id, insertData)
+monacoSetOptions(id, options)
}
class MonacoEditor {
+getSelection()
+executeEdits(source, edits)
+focus()
}
class DataStore {
+get(id)
}
CodeEditorComponent --> CodeEditor_razor_js : uses_via_JSInterop
CodeEditor_razor_js --> DataStore : uses
CodeEditor_razor_js --> MonacoEditor : manipulates
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- In
insertText, consider guarding against missingwrapper.editororeditor.getSelection()returning null, and explicitly updating the selection afterexecuteEditsto address the cursor-position bug noted in the comment. - In the C#
InsertTextmethod, you may want to skip invoking JS whendatais null or empty to avoid unnecessary interop calls and potential JS-side edge cases.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `insertText`, consider guarding against missing `wrapper.editor` or `editor.getSelection()` returning null, and explicitly updating the selection after `executeEdits` to address the cursor-position bug noted in the comment.
- In the C# `InsertText` method, you may want to skip invoking JS when `data` is null or empty to avoid unnecessary interop calls and potential JS-side edge cases.
## Individual Comments
### Comment 1
<location> `src/components/BootstrapBlazor.CodeEditor/Components/CodeEditor/CodeEditor.razor.cs:125` </location>
<code_context>
+ /// </summary>
+ /// <param name="data"></param>
+ /// <returns></returns>
+ public async Task InsertText(string data) => await InvokeVoidAsync("insertText",Id,data);
+
/// <summary>
</code_context>
<issue_to_address>
**suggestion:** Consider null/empty input handling for `InsertText` to avoid unnecessary JS interop calls
If `data` can be `null` or empty, consider short-circuiting with `string.IsNullOrEmpty(data)` so you avoid a no-op interop call or having to add extra defensive checks on the JS side (unless the JS layer is explicitly expected to validate this).
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| /// </summary> | ||
| /// <param name="data"></param> | ||
| /// <returns></returns> | ||
| public async Task InsertText(string data) => await InvokeVoidAsync("insertText",Id,data); |
There was a problem hiding this comment.
suggestion: Consider null/empty input handling for InsertText to avoid unnecessary JS interop calls
If data can be null or empty, consider short-circuiting with string.IsNullOrEmpty(data) so you avoid a no-op interop call or having to add extra defensive checks on the JS side (unless the JS layer is explicitly expected to validate this).
Contributor
|
@kimdiego2098 测试没有发现问题 |
Contributor
Author
|
@Vision-Zhang 已添加复现demo |
Member
|
@kimdiego2098 复现 demo 是啥？完全看不懂，能否写一下操作步骤，期望值是啥？实际是啥？差异在哪里，修复了什么？还是增加了 insertText 功能？但是我测试一下功能也不正常，光标一直跑到最前面去，不知道是不是故意这样设计的。 还有复现的工程不要放到 PR 里，这会导致没有办法合并也增加了 review 难度？弄个 zip 上传即可 |
ArgoZhang
changed the title
This reverts commit 3ad5529.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issues
close #903
feat: 增加代码编辑器插入文本方法
Summary by Sourcery
Add a new API to insert custom text into the code editor at the current selection via JS interop.
New Features: